KFLUXINFRA-4167: Create a staging ArgoCD instance for infra-deployments#532
KFLUXINFRA-4167: Create a staging ArgoCD instance for infra-deployments#532enkeefe00 wants to merge 1 commit into
Conversation
|
@enkeefe00: This pull request references KFLUXINFRA-4167 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: enkeefe00 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
PR Summary by QodoAdd GitOps-managed ArgoCD instance for infra-deployments (staging/prod)
AI Description
Diagram
High-Level Assessment
Files changed (29)
|
enkeefe00
force-pushed
the
create-infra-deployments-argocds
branch
4 times, most recently
from
5c65f04 to
3b01561
Compare
|
/agentic_review |
Code Review by Qodo
Context used✅ Compliance rules (platform):
5 rules 1.
|
![qodo-for-redhat-appstudio[bot]](https://avatars.githubusercontent.com/u/89927105?s=60&v=4){kind=small}
|
Code review by qodo was updated up to the latest commit 3b01561 |
enkeefe00
force-pushed
the
create-infra-deployments-argocds
branch
from
3b01561 to
9c52c72
Compare
|
/agentic_review |
|
Code review by qodo was updated up to the latest commit 9c52c72 |
enkeefe00
force-pushed
the
create-infra-deployments-argocds
branch
from
9c52c72 to
fcc3132
Compare
|
/agentic_review |
|
Code review by qodo was updated up to the latest commit fcc3132 |
enkeefe00
force-pushed
the
create-infra-deployments-argocds
branch
from
fcc3132 to
0969f42
Compare
enkeefe00
force-pushed
the
create-infra-deployments-argocds
branch
2 times, most recently
from
ed80f8d to
a494b81
Compare
|
/agentic_review |
|
Code review by qodo was updated up to the latest commit a494b81 |
enkeefe00
force-pushed
the
create-infra-deployments-argocds
branch
from
a494b81 to
dabc830
Compare
|
/agentic_review |
|
Code review by qodo was updated up to the latest commit dabc830 |
|
@enkeefe00: This pull request references KFLUXINFRA-4167 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
enkeefe00
changed the title
| apiVersion: argoproj.io/v1alpha1 | ||
| kind: ApplicationSet | ||
| metadata: | ||
| name: argocd-infra-deployments-instance |
There was a problem hiding this comment.
I am curious, why adding the word "instance"? if we feel that this is needed, then everything should be name -instance, like internal-services-instance, kargo-instance and so on.
If you have good argument for keeping it, we can but I would like to understand. I would personally name the applicationSet just "argocd-infra-deployments"
| generators: | ||
| - clusters: | ||
| values: | ||
| sourceRoot: components/argocd-infra-instance |
There was a problem hiding this comment.
we usually name the ApplicationSet, component folder and namespace the same. We should follow this here too so depending on the applicationSet we agree above, I think this component folder should be renamed too, to what ever name we agree above.
| repoURL: https://github.com/redhat-appstudio/infra-common-deployments.git | ||
| targetRevision: main | ||
| destination: | ||
| namespace: argocd-infra-instance |
There was a problem hiding this comment.
we usually name the ApplicationSet, component folder and namespace the same. We should follow this here too so depending on the applicationSet we agree above, I think this namespace should be renamed too, to what ever name we agree above.
| @@ -0,0 +1,9 @@ | |||
| apiVersion: kustomize.config.k8s.io/v1beta1 | |||
| kind: Kustomization | |||
| namespace: argocd-infra-deployments-staging | |||
There was a problem hiding this comment.
I am a bit confused here, we use yet another NS than the target one from applicationSet?
There was a problem hiding this comment.
I changed the applicationSet to name the namespace using the environment variable.
| @@ -0,0 +1,266 @@ | |||
| apiVersion: argoproj.io/v1beta1 | |||
| kind: ArgoCD | |||
There was a problem hiding this comment.
where did check to get this CR populated, I see difference between this one and the one we use in app-interface to create the our staging ArgoCD
There was a problem hiding this comment.
I originally used what we were using in the ArgoCD Terraform module. I updated it to what I found in app-interface.
| - extract: | ||
| conversionStrategy: Default | ||
| decodingStrategy: None | ||
| key: staging/infrastructure/github-argocd/kflux-c-stg-i01/redhat-appstudio |
There was a problem hiding this comment.
here, we should not reuse a repo secret from another cluster, we should have one for this ArgoCD. We have 3 options, use the one from existing internal Argo, use the existing one from external Argo, or create another one for this instance and eventually that will be the only one left once we pull the plug on 2 former ArgoCD.
I vote for option 3 so create secret in vault with this path and use it: stonesoup/staging/platform/github-argocd/argocd-infra-deployments (again, we want to name this base on applicationSet/Application/NS/component so better sort out the naming so we stay consistent)
enkeefe00
force-pushed
the
create-infra-deployments-argocds
branch
from
dabc830 to
41f5685
Compare
Use GitOps to deploy ArgoCD instances for infra-deployments KFLUXINFRA-4167
enkeefe00
force-pushed
the
create-infra-deployments-argocds
branch
from
41f5685 to
455a49d
Compare
| apiVersion: rbac.authorization.k8s.io/v1 | ||
| kind: ClusterRole | ||
| metadata: | ||
| name: argocd-infra-deployments-custom-permissions |
There was a problem hiding this comment.
do we need that? This argoCD will create resource in a remote cluster using the permission allowed to the serviceAccount used by argo, specified in the k8s secret.
| labels: | ||
| app.kubernetes.io/name: argocd-infra-deployments-staging | ||
| app.kubernetes.io/part-of: konflux-common | ||
| app.kubernetes.io/instance: argocd-infra-deployments-staging |
There was a problem hiding this comment.
why do we need those 3 labels?
| repoURL: https://github.com/redhat-appstudio/infra-common-deployments.git | ||
| targetRevision: main | ||
| destination: | ||
| namespace: argocd-infra-deployments-{{values.environment}} |
There was a problem hiding this comment.
this will end up being "argocd-infra-deployments-internal-staging" and not "argocd-infra-deployments-staging" as expected in the rest of this PR.
If you want the suffix staging in the NS name, you will need to use go templating like we did in k8s-groups applicationSet
spec:
goTemplate: true
....
and then this would be:
argocd-infra-deployments-{{trimPrefix "external-" (trimPrefix "internal-" .values.environment)}}
Question, we will have one argocd on common staging cluster for the staging konflux clusters and one on common production cluster for the production konflux clusters, another option is just use argocd-infra-deployments as NS as there will no t be any conflict.
| p, role:release-eng, applications, get, rh-managed-workspaces-config/*, allow | ||
| p, role:release-eng, logs, get, rh-managed-workspaces-config/*, allow | ||
|
|
||
| g, argocd-release-eng, role:release-eng |
There was a problem hiding this comment.
I want to highlight that this will not work. Here we configure argocd permission and map them to the k8s groups "argocd-developers" and "argocd-release-eng". Those 2 groups exist on appsre cluster, created by some yaml in app-interface but they do not in the common cluster.
First, we do not need the tenants-config and releng permissions, this is a left over from when we had single argocd instance deploying both tenant and konflux itself.
For the developer role, that we still need, we could probably do like we did in the local arogcd, i.e. bind all the konflux-* groups to the dev people:
p, role:konflux-argocd-devs, applications, get, */*, allow
p, role:konflux-argocd-devs, applications, sync, */*, allow
p, role:konflux-argocd-devs, logs, get, */*, allow
g, konflux-admins, role:konflux-argocd-devs
g, konflux-build, role:konflux-argocd-devs
g, konflux-contributors, role:konflux-argocd-devs
g, konflux-devprod, role:konflux-argocd-devs
g, konflux-ec, role:konflux-argocd-devs
g, konflux-infra, role:konflux-argocd-devs
g, konflux-integration, role:konflux-argocd-devs
g, konflux-kubearchive, role:konflux-argocd-devs
g, konflux-migration, role:konflux-argocd-devs
g, konflux-mintmaker-team, role:konflux-argocd-devs
g, konflux-o11y, role:konflux-argocd-devs
g, konflux-qe, role:konflux-argocd-devs
g, konflux-release-team, role:konflux-argocd-devs
g, konflux-support-ops, role:konflux-argocd-devs
g, konflux-ui, role:konflux-argocd-devs
g, konflux-vanguard, role:konflux-argocd-devs
3 participants
Use GitOps to deploy a staging ArgoCD instance for infra-deployments